Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: Move Player ghosting functionality to GhostComponent #1413

Merged
merged 10 commits into from
Jan 14, 2024

Conversation

EmosewaMC
Copy link
Collaborator

Waiting on #1412
Tested that ghosting still works and players are still firing off the OnPlayerLeave and relevant handlers.

- Move code to CharacterComponent
- Remove extraneous interfaces
- Simplify some code greatly
- Change some types to return and take in const ref (only structs larger than 8 bytes benefit from this change.)
- Update code to use CharacterComponent for sending to zone instead of Player*.
- Move code to CharacterComponent
- Remove extraneous interfaces
- Simplify some code greatly
- Change some types to return and take in const ref (only structs larger than 8 bytes benefit from this change.)
- Update code to use CharacterComponent for sending to zone instead of Player*.
- Remove static storage container (static containers can be destroyed before exit/terminate handler executes)
Used for the static Player functions.  Further removes stuff from the Player class/file.
Tested that ghosting still works and players are still firing off the OnPlayerLeave and relevant handlers.
aronwk-aaron
aronwk-aaron previously approved these changes Jan 13, 2024
Copy link
Member

@Wincent01 Wincent01 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Change to ghosting vector to a hashset.

@EmosewaMC
Copy link
Collaborator Author

Change to ghosting vector to a hashset.

Are you sure you would like to integrate a change like this into this PR? I tried to make this one as close to just moving the code as possible and updating the calls where necessary. Is this still a change you would like?

Copy link
Member

@Wincent01 Wincent01 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Felt like an appropriate time to get that change through; shouldn't have phrased it as a requirement though.

@EmosewaMC
Copy link
Collaborator Author

Felt like an appropriate time to get that change through; shouldn't have phrased it as a requirement though.

will do so then, just wanted to double check since this seemed to be optimized for storage and cache locality for some reason. Not sure if that was for speed or because unordered_set was unknown about at the time

@Wincent01
Copy link
Member

This was written before I'd taken the data structures course at uni lol

Wincent01
Wincent01 previously approved these changes Jan 13, 2024
aronwk-aaron
aronwk-aaron previously approved these changes Jan 14, 2024
@aronwk-aaron
Copy link
Member

aronwk-aaron commented Jan 14, 2024

conflicts

:(

@EmosewaMC EmosewaMC dismissed stale reviews from aronwk-aaron and Wincent01 January 14, 2024 01:37

The merge-base changed after approval.

@EmosewaMC
Copy link
Collaborator Author

Change to ghosting vector to a hashset.

I love the prospect of "Please get rid of my old code now, we dont need it anymore"

@aronwk-aaron aronwk-aaron merged commit c83ec82 into main Jan 14, 2024
4 checks passed
@aronwk-aaron aronwk-aaron deleted the player-stuff-3 branch January 14, 2024 19:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants